Skip to content

cli: Derive several shares per run and allow repeated indices - #103

Draft
BenWestgate wants to merge 2 commits into
30-recorded-fingerprint-gatefrom
claude/new-issue-fixes-fvhbl3-87
Draft

BenWestgate wants to merge 2 commits into
30-recorded-fingerprint-gatefrom
claude/new-issue-fixes-fvhbl3-87

Conversation

@BenWestgate

@BenWestgate BenWestgate commented Oct 1, 2026 •

Copy link
Copy Markdown
Owner

Requested by Ben · project thread

Before: ms32 share INDEX derived one share per run, so three new cards meant entering the existing cards three times. An index matching an entered card was rejected, and create --indices rejected repeats.

After: ms32 share cdf takes the existing cards once, then writes and confirms D, C and F in turn. An index that matches an entered card copies it, and if every requested index matches an entered card, only those cards are needed. A repeated index makes another copy, in share and create --indices, and repeats run after every original: aacd runs a, c, d, a. S is still rejected and share has no random count.

How: _indices in generation.py keeps repeats and orders them in rounds. _selection counts distinct indices against the threshold. CreationCeremony.next_share copies a repeated random card from its basis instead of deriving it. _share_command loops over the requested indices, and interactive entry stops early once the requested indices are all entered cards. The old excluded-index check is gone.

Security review: Codex Security reviewed the behavior-changing head d9ce204 on 2026-10-04 with complete coverage of the five changed production files and no reportable finding. The follow-up 2924f5f changes only the public contract/docs and constructor docstrings to resolve the automated review's ordering mismatch: repeated output indices are ordered after their first occurrence, creation still requires threshold distinct initial indices, copies come from already-confirmed basis cards, and any requested new index still goes through derive_share() validation before any batch output is printed. S remains excluded and the 31-output bound remains enforced.

Budget: 5,198 of <5,200. The behavior-changing head passed exact-head GitHub Python-package CI; 927 tests passed there, with Ruff, mypy, and the touched files under -O clean. The documentation-only follow-up keeps git diff --check clean and remains inside the authorized library cap.

Overlaps #100 in _share_command: when both land, print the fingerprint only when a threshold was entered, since a copy-only run can't recover the secret.

Closes #87

🤖 Generated with Claude Code

https://claude.ai/code/session_015CuLXqAvAovfoVcUmmogwa


Generated by Claude Code

@BenWestgate BenWestgate self-assigned this Oct 1, 2026
@BenWestgate
BenWestgate force-pushed the 30-recorded-fingerprint-gate branch from 37eef4d to a7efaae Compare October 1, 2026 18:22
@BenWestgate
BenWestgate force-pushed the claude/new-issue-fixes-fvhbl3-87 branch from 10e46b4 to 90de3ef Compare October 1, 2026 19:35
@BenWestgate
BenWestgate force-pushed the 30-recorded-fingerprint-gate branch 2 times, most recently from 054e8d9 to 115f2c2 Compare October 2, 2026 08:57
`ms32 share INDEX` derived one share per run, so adding three cards
meant entering the existing cards three times.

`share` now takes several indices, such as `share cdf`: the operator
enters the existing cards once, then writes and confirms each new card.
An index that matches an entered card makes a copy of it, and if every
requested index matches an entered card, those cards are all it needs.
`S` is still rejected and `share` gets no random count.

A repeated index makes another copy, in `share` and in
`create --indices`. Repeats move after every original (aacd runs a, c,
d, a), so the same card is never confirmed twice in a row. In `create`
the random cards therefore always have distinct indices, a repeat copies
the card already confirmed, and the threshold check counts distinct
indices.

Every requested card is resolved before any is printed, so a request
that cannot be completed prints nothing rather than part of a set. The
share-count error now says string(s) instead of artifacts.

Closes #87

Claude-Session: https://claude.ai/code/session_015CuLXqAvAovfoVcUmmogwa
@BenWestgate
BenWestgate force-pushed the claude/new-issue-fixes-fvhbl3-87 branch from 90de3ef to d9ce204 Compare October 2, 2026 17:20

@BenWestgate BenWestgate left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AI-generated review (Codex), posted at the maintainer's request.

ACK d9ce204. Multi-index derivation and repeated-index copies preserve threshold validation and avoid partial output on failure.

@BenWestgate
BenWestgate marked this pull request as ready for review October 4, 2026 17:41

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d9ce204f2f

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/codex32/generation.py
@BenWestgate BenWestgate added area: cli Command-line interface behavior. area: security Security invariants, hardening, and security-sensitive boundaries. gate: adversarial review Resolve, merge, or explicitly defer before the next full adversarial review. labels Oct 4, 2026

@BenWestgate BenWestgate left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AI-generated focused security review (Codex), posted at the maintainer's request.

Security ACK d9ce204f2f6f for the share-generation/repeated-index boundary.

I checked the two invariants called out in the PR body:

  • _indices() orders first occurrences before later copies, and _selection() counts distinct indices, so the first threshold direct cards of a fresh ceremony remain distinct entropy-bearing shares. CreationCeremony.next_share() re-emits the already-confirmed basis card for a repeated index instead of generating a second random share with the same index.
  • share INDICES may stop interactive input early only when every requested index has been entered. Copy-only output therefore re-emits validated entered cards. If any requested index is new, _share_command() still calls derive_share() and its share-set validation before any output is printed; one failure suppresses the whole output batch.

S remains excluded by the ordinary-index parser, the 31-output bound remains in _indices(), and the new tests exercise repeated ordering/copy identity plus multi-output/no-partial-output behavior.

No security blocker found in this diff. Human review/authorship policy still applies before integration.

Copy link
Copy Markdown
Owner Author

V1 disposition (Codex, 2026-10-04): defer this post-audit share-generation enhancement from the v1 candidate.

Current head d9ce204 has both a code ACK and a focused security ACK: repeated indices remain copies, threshold distinctness is preserved, copy-only output does not derive from an under-threshold basis, and batch failure emits no partial card set. The reason to defer is release scope, not a security defect.

#103 materially expands share-generation/output semantics late in the v1 freeze and overlaps #100 in _share_command. None of the validated DeepSeek/GLM/Kimi/consolidated audit findings requires multi-index derivation or repeated-index copies. Keep the reviewed PR open for post-v1 integration, where the overlap can be resolved without consuming the v1 review budget or destabilizing the frozen candidate. Human authorship/review remains required when it is eventually integrated.

@BenWestgate
BenWestgate marked this pull request as draft October 4, 2026 19:18
@BenWestgate BenWestgate removed the gate: adversarial review Resolve, merge, or explicitly defer before the next full adversarial review. label Oct 4, 2026
Document that repeated explicit indices are physical copies emitted after their first occurrences, while preserving first-occurrence order. Mirror that contract in the public CreationCeremony constructor docstrings so callers do not associate outputs with stale positional semantics.\n\nRefs #87.

@BenWestgate BenWestgate left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AI-generated current-head follow-up review (Codex), performed at the maintainer's request.

ACK 2924f5f53773. The behavior-changing parent d9ce204 already has a focused security ACK with no reportable finding. The only newer commit documents the reviewed repeated-index contract: first occurrences precede later physical copies, the ceremony still requires threshold distinct entropy-bearing indices, and copies reuse confirmed basis cards. That resolves the prior ordering/documentation mismatch without changing runtime behavior. Exact-head Python-package run 760 succeeded.

No remaining automated security/code-review blocker on this head. Responsible-human review/authorship policy still applies before integration.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: cli Command-line interface behavior. area: security Security invariants, hardening, and security-sensitive boundaries.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants